Jonyscathe/execute mutate #342

jonyscathe · 2023-04-18T07:13:23Z

This adds support for modifying mock_data through any of:

session.execute(insert(my_table).values(<list of dicts to insert>))  # similar to session.add()
session.execute(insert(my_table), <list of dicts to insert>)  # similar to session.add()
session.execute(insert(my_table).returning(my_table), <list of dicts to insert>).scalar()  # similar to session.add(), in theory should return ORM object, in practice, don't quite have that one working yet... Not terribly simple to do, and not needed in my project.
session.execute(update(my_table).where(<some expression).values(<dict of values to update>))  # Will update data in self._mock_data as appropraite
session.execute(delete(my_table).where(<some expression))  # will delete data in self._mock_data as appropriate.

This allows for the mocking of ORM 2.0 style expressions .

Tests and documentation has not been updated yet, and I am not sure that this will not cause any runtime issues with sqlalchemy < 1.4.0.
I wanted to get feedback on this first before going to the effort to update tests/documentation/compatibility.

If you are happy with these changes then I'll update the tests/doco/etc.

Note: I currently have this working in a project that has 185 various session.execute() calls that previously had 185 various session.query() or session.add() calls. Mocking is working well, and the ability to actually update mocked data has been particularly useful as I wasn't able to do that previously.

Can now add data and read it back using session.execute(select(...))

For session.execute(update(...)) statements and session.execute(delete(...)) statements

As an alternative to using session.add()

Still no tests cases or linting done on this, very much work in progress. But it does seem to work ok

Manually throw in a boundary if an execute(insert()) is not returning

Will hopefully finish this next week sometime

ORM 2.0 is pretty useable now. Still needs tests

On no rows to update or delete

Undo changes I had made for testing

jonyscathe · 2023-04-25T22:49:24Z

Any comments on whether or not this change is wanted?

It isn't a huge amount of work for me to get pre-commit passing, add tests, add documentation. But don't want to spend my time doing that if you aren't ok with the change itself.

rajivsarvepalli

Thanks for the pull-request to add this feature.

One overall comment would be to add unit testing so that we can verify these changes and then we can collaborate on a separate PR to enhance documentation for this feature so that users can leverage this new feature.

Have you noticed any regression issues with these changes? It appears the current unit tests are passing so it looks these changes should function fine for existing users,

rajivsarvepalli · 2023-05-14T20:43:22Z

src/mock_alchemy/mocking.py

@@ -685,3 +703,100 @@ def _mutate_data(self, *args: Any, **kwargs: Any) -> Optional[int]:
                    temp_mock_data.append((calls, result))
            self._mock_data = temp_mock_data
            return num_deleted
+        # execute case
+        else:


Is there an opportunity here to break to down this new logic into a new function?

rajivsarvepalli · 2023-05-14T20:47:31Z

src/mock_alchemy/mocking.py

+                # TypeError indicates an old version of sqlalcemy that does not support
+                # executing select(table) statements.
+                # ArgumentError indicates a mocked table that cannot be selected, so
+                # cannot be mocked this way.


For ArgumentError shouldn't we inform the user that the mocked table could not be selected?

For TypeError , can we write this logic to a statement that is supported by multiple SqlAlchemy versions or do a check to verify the version supports executing select(table) statements rather relying on a exception to catch this case?

rajivsarvepalli · 2023-05-14T20:49:39Z

src/mock_alchemy/mocking.py

-    mutate: Set[str] = {"add", "add_all", "delete"}
+    mutate: Set[str] = {"add", "add_all", "delete", "execute"}
+
+    mutate_and_unify: Set[str] = {"execute"}


[FMI] Why is mutate and unify? Shouldn't this be mutate or unify?

Also do we need execute to be part of mutate as well?

Good point.
Execute should just be its own special case. Which can mutate data and potentially unify depending on what is being executed.

Also return data from an insert with a returning statement.

jonyscathe · 2023-05-16T08:19:34Z

Thanks for the pull-request to add this feature.

One overall comment would be to add unit testing so that we can verify these changes and then we can collaborate on a separate PR to enhance documentation for this feature so that users can leverage this new feature.

Have you noticed any regression issues with these changes? It appears the current unit tests are passing so it looks these changes should function fine for existing users,

Added unit tests. Now back to 100% test coverage and pre-commit passing.
For tests I took test_mocking.py and made a similar file test_mocking_orm2.py and changed the code over to orm2 style and removed anything that was irrelevant or not supported.

I haven't added any support for session.get() or session.scalars(). See https://docs.sqlalchemy.org/en/20/changelog/migration_20.html#migration-orm-usage for info.
scalars() would be fairly easy to add. get() would be harder as we would need to distinguish between something like session.query(User).get(42) and something like session.get(User, 42) which should both give the same result, but are quite different syntax.

For an insert that returns data, currently I have it returning all data for that table rather than what was just inserted... So technically it isn't quite right. Still thinking about how to make it only return the row(s) inserted in that instruction.

jonyscathe added 9 commits April 14, 2023 11:30

feat: Allow data mutation with ORM 2.0

cac2007

Can now add data and read it back using session.execute(select(...))

fix: Allow more expression matches

e7bd57f

For session.execute(update(...)) statements and session.execute(delete(...)) statements

chore: Add insert as well

bd9adf0

As an alternative to using session.add()

feat: execute(insert()) insterts data

7495ff3

Still no tests cases or linting done on this, very much work in progress. But it does seem to work ok

chore: Insert a boundary

045a98f

Manually throw in a boundary if an execute(insert()) is not returning

chore: Minor tidy up and todos

a40ccc1

Will hopefully finish this next week sometime

feat: Added support for delete/upate

5797ec7

ORM 2.0 is pretty useable now. Still needs tests

chore: handle zero case

7e8d3e5

On no rows to update or delete

chore: Change noxfile back

bc422a7

Undo changes I had made for testing

rajivsarvepalli reviewed May 14, 2023

View reviewed changes

chore: Add tests

b68a236

Also return data from an insert with a returning statement.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jonyscathe/execute mutate #342

Jonyscathe/execute mutate #342

jonyscathe commented Apr 18, 2023

jonyscathe commented Apr 25, 2023

rajivsarvepalli left a comment

rajivsarvepalli May 14, 2023

rajivsarvepalli May 14, 2023

rajivsarvepalli May 14, 2023

jonyscathe May 16, 2023

jonyscathe commented May 16, 2023

Jonyscathe/execute mutate #342

Are you sure you want to change the base?

Jonyscathe/execute mutate #342

Conversation

jonyscathe commented Apr 18, 2023

jonyscathe commented Apr 25, 2023

rajivsarvepalli left a comment

Choose a reason for hiding this comment

rajivsarvepalli May 14, 2023

Choose a reason for hiding this comment

rajivsarvepalli May 14, 2023

Choose a reason for hiding this comment

rajivsarvepalli May 14, 2023

Choose a reason for hiding this comment

jonyscathe May 16, 2023

Choose a reason for hiding this comment

jonyscathe commented May 16, 2023